Skip to content

Use new handles API in ros2_controllers to fix deprecation warnings #1566

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 21 commits into
base: master
Choose a base branch
from

Conversation

kumar-sanjeeev
Copy link
Contributor

@kumar-sanjeeev kumar-sanjeeev commented Mar 3, 2025

This PR applies the new API changes in the handles of ros2_control in following controllers in same fashion as #1565:

  • ackermann_steering_controller
  • bicycle_steering_controller
  • admittance_controller
  • effort_controllers
  • forward_command_controller
  • gpio_controllers
  • gripper_controllers
  • mecanum_drive_controller
  • parallel_gripper_controller
  • tricycle_controller
  • pid_controller
  • tricycle_steering_controller
  • mecum_driver_contoller
  • force_torque_sensor_broadcaster
  • joint_state_broadcaster

Copy link
Contributor

@christophfroehlich christophfroehlich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine, thanks!

@kumar-sanjeeev
Copy link
Contributor Author

Looks fine, thanks!

@christophfroehlich hi, one question - I have made progress on other controllers as well locally on my system. Can I push these changes into this PR as well (will update description), or should I wait for more reviews on current submitted PR ?

@christophfroehlich
Copy link
Contributor

@saikishor ?

Copy link
Member

@saikishor saikishor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes already look good to me. I just forgot to comment.
@kumar-sanjeeev yes, please proceed with other packages as well

@kumar-sanjeeev
Copy link
Contributor Author

@saikishor and @christophfroehlich , one question, would you like me to complete all the controller packages first, and then you would review them of all together? Or would you prefer to first provide feedback on the commits that have already been pushed in this PR?

@christophfroehlich
Copy link
Contributor

apply the changes to all of them please

@saikishor
Copy link
Member

Yes please

@christophfroehlich
Copy link
Contributor

@kumar-sanjeeev I haven't crosschecked your list, are there all controllers included now? (you might add a checklist of all controllers and just check the finished ones)

@kumar-sanjeeev
Copy link
Contributor Author

@kumar-sanjeeev I haven't crosschecked your list, are there all controllers included now? (you might add a checklist of all controllers and just check the finished ones)

hi @christophfroehlich
I am almost done, just a few controllers are pending. I will update the list as checklists (nice suggestion :) )

@christophfroehlich
Copy link
Contributor

hi @kumar-sanjeeev will you have time to finish this PR? otherwise, I'd like to review and merge the current status and fix he remaining ones in a future PR.

@kumar-sanjeeev
Copy link
Contributor Author

@christophfroehlich Sorry, I was a bit busy. give me time until next weekend, I’ll try to finish this PR by then, and you can review it afterward. If it’s urgent, feel free to start reviewing and leave comments where improvements are needed. I’ll address those along with the pending controllers that require fixes.

@bmagyar
Copy link
Member

bmagyar commented May 3, 2025

Thank you, @kumar-sanjeeev , please keep an eye on the current conflict too ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants